fix(ExpectedConditions): allow ExpectedConditions to handle missing e… #3972
Conversation
| @@ -210,7 +210,7 @@ export class ProtractorExpectedConditions { | |||
| // MSEdge does not properly remove newlines, which causes false | |||
| // negatives | |||
| return actualText.replace(/\r?\n|\r/g, '').indexOf(text) > -1; | |||
| }); | |||
| }, () => false); | |||
There was a problem hiding this comment.
Should we be more specific about which errors we mask here?
| return this.elementArrayFinder_.then((actionResults: any) => { | ||
| return this.elementArrayFinder_.then(null, (error) => { | ||
| if (elementArrayFinder_.falseIfMissing_ && | ||
| (error instanceof wderror.NoSuchElementError)) { |
There was a problem hiding this comment.
Do we also need to handle StaleElement exceptions?
|
This has also come up in #3578 (comment), and there's an npm package out there to patch around the issue: https://github.com/tilmanpotthof/protractor-save-expected-conditions In that 'protractor-save-expected-conditions' package, he masks NoSuchElement and StaleElement in edit: Nevermind, I misread what he was doing. I do think we need to also catch StaleElement errors for the "false if missing" actions. That and the |
|
Also we could probably simplify |
…lements Expected conditions used `presenceOf` and `visibilityOf` to check that it's referencing elements which actually exist on the page, but there is a race condition with this strategy: an element could disappear after the `presenceOf`/`visibilityOf` check but before other checks, causing an error to be thrown. This PR handles this race condition in two ways: 1. `ElementFinder`'s `isEnabled`, `isDisplayed`, and `isSelected` functions now return false if no such element exists, rahter than throwing an error 2. `ExpectedConditions`'s `textToBePresent` and `textToBePresentInElementValue` now check for errors and also return false in those cases This is a general solution to the problem referenced in angular#3777 and angular#3958.
|
@mgiambalvo all comments addressed |
|
ack, I forgot to run tests. Give me a second to debug this |
|
@mgiambalvo Hi, the problem in the expected conditions is not the The possibility of these race conditions is very low for a single test, but I had it in almost every build with >600 test in 2-10 random test. This started after I replaced all |
…ibility (#4006) Add test cases to reproduce the missing element race conditions possible in expected condition methods `visibilityOf`, `textToBePresentInElement`, `textToBePresentInValue` and `elementToBeClickable`. Add error handler `falseIfMissing` to all expected conditions that depend on the presence of an element. Expected conditions check the presence of an element before other checks, but when an element is removed exactly in the moment after the `isPresent` and before `isDisplayed` in `visibilityOf` the condition used to fail. This solution does not handle missing elements in (`isEnable`, `isDisplayed`, `isSelected`) and focused only on expected conditions (see #3972) This problem was also referenced in #3578 and #3777
|
Closing this as superceded by #4006 |
…ibility (angular#4006) Add test cases to reproduce the missing element race conditions possible in expected condition methods `visibilityOf`, `textToBePresentInElement`, `textToBePresentInValue` and `elementToBeClickable`. Add error handler `falseIfMissing` to all expected conditions that depend on the presence of an element. Expected conditions check the presence of an element before other checks, but when an element is removed exactly in the moment after the `isPresent` and before `isDisplayed` in `visibilityOf` the condition used to fail. This solution does not handle missing elements in (`isEnable`, `isDisplayed`, `isSelected`) and focused only on expected conditions (see angular#3972) This problem was also referenced in angular#3578 and angular#3777
…ibility (#4006) Add test cases to reproduce the missing element race conditions possible in expected condition methods `visibilityOf`, `textToBePresentInElement`, `textToBePresentInValue` and `elementToBeClickable`. Add error handler `falseIfMissing` to all expected conditions that depend on the presence of an element. Expected conditions check the presence of an element before other checks, but when an element is removed exactly in the moment after the `isPresent` and before `isDisplayed` in `visibilityOf` the condition used to fail. This solution does not handle missing elements in (`isEnable`, `isDisplayed`, `isSelected`) and focused only on expected conditions (see angular/protractor#3972) This problem was also referenced in angular/protractor#3578 and angular/protractor#3777
…lements
Expected conditions used
presenceOfandvisibilityOfto check that it'sreferencing elements which actually exist on the page, but there is a race
condition with this strategy: an element could disappear after the
presenceOf/visibilityOfcheck but before other checks, causing an errorto be thrown. This PR handles this race condition in two ways:
ElementFinder'sisEnabled,isDisplayed, andisSelectedfunctions nowreturn false if no such element exists, rahter than throwing an error
ExpectedConditions'stextToBePresentandtextToBePresentInElementValuenow check for errors and also return false in those cases
This is a general solution to the problem referenced in
#3777 and
#3958.
I'm assigning @juliemr, since it's a low level change, and @mgiambalvo, since we discussed this issue earlier today